Avoid division in round_up #1343
Conversation
lib/libthr/thread/thr_stack.c
Outdated
| size = ((size / _thr_page_size) + 1) * | ||
| _thr_page_size; | ||
| return size; | ||
| return (size + _thr_page_size - 1) & ~(size_t)(_thr_page_size - 1); |
There was a problem hiding this comment.
Any reason not to use roundup2(size, _thr_page_size)? It's either a compiler buildit or the same code.
There was a problem hiding this comment.
I forgot about roundup2. Konstantin Belousov sent a diff to the mailing lists using that function.
Division may require a function call, leading to dynamic function lookup, leading to deadlock in rtld.
Division may require a function call, leading to dynamic function lookup, leading to deadlock in rtld. Pull Request: #1343
lib/libthr/thread/thr_stack.c
Outdated
| size = ((size / _thr_page_size) + 1) * | ||
| _thr_page_size; | ||
| return size; | ||
| return (roundup2(size, _thr_page_size - 1)); |
There was a problem hiding this comment.
| return (roundup2(size, _thr_page_size - 1)); | |
| return (roundup2(size, _thr_page_size)); |
I'd merged this, but Doug Moore correctly pointed out that this is wrong.
There was a problem hiding this comment.
You're right. Too many versions of code going around and I stopped thinking about each new one. It will usually work, but the - 1 is wrong.
There was a problem hiding this comment.
I pushed a fix to the branch associated with this pull request.
not the mask equal to page size - 1.
Division may require a function call, leading to dynamic function lookup, leading to deadlock in rtld. Pull Request: freebsd#1343
|
FYI this was reverted in f5c8944 as: |
kib beat to to remerging it with 31f688a |
Round up to page size using add and mask instead of divide and multiply. This is slightly faster, but more importantly it avoids a function call when division is not implemented in hardware. Looking up the target of the call here can lead to deadlock in rtld. See recent discussion on -arm and -current.
Some older armv7 CPUs do not have hardware divide. Theoretically FreeBSD could also target a RISC-V CPU without hardware multiply and divide.